Skip to content

fix: Instant Book: Cleans up issue with caching#433

Merged
tuj merged 18 commits into
release/3.0.0from
feature/instantbook-cleanup
May 21, 2026
Merged

fix: Instant Book: Cleans up issue with caching#433
tuj merged 18 commits into
release/3.0.0from
feature/instantbook-cleanup

Conversation

@tuj

@tuj tuj commented May 6, 2026

Copy link
Copy Markdown
Contributor

Link to isse

#436

Link to ticket

https://leantime.itkdev.dk/#/tickets/showTicket/7208

Description

Cleans up issue with Instant Book caching and fixed other bugs.

Checklist

  • My code is covered by test cases.
  • My code passes our test (all our tests).
  • My code passes our static analysis suite.
  • My code passes our continuous integration process.

Changes

  • Controller uses a typed DTO:

    • InteractiveController reads InteractiveSlideActionInput from request->attributes->get('data') instead of $request->toArray().
    • DTO gains implementationClass; data is now ?array.
    • Service method renamed parseRequestBody(array) → parseInteractiveSlideActionInput(InteractiveSlideActionInput).
    • API spec + RTK type regenerated.
  • InstantBook bug fixes

    • intervalFree boundary fix: >/< → >=/<= so adjacent bookings don't count as overlap.
    • getBusyIntervals end-time was using start-time's timezone — now uses eventEnd['timeZone'].
    • Spam-protect key changed from slide->getId() to $resource (per-resource throttling).
    • Spam-protect logic was always-true (now+ttl > now); rewritten with a &$isFreshRequest flag set inside the cache miss callback.
    • durationMinutes validated against [15, 30, 60] instead of being passed straight to DateInterval.
    • Booking response: HTTP 409 → ConflictException, any >= 400 → NotAcceptableException (was silently returning the failed status).
    • CACHE_KEY_RESOURCES now actually expires (PT5M); was being cached forever.
    • Property typo keyValueService → keyVaultService.
    • Exception-message string interpolation fix in getValueFromInterval.
  • Performance / cache

    • New getBusyIntervals cache (PT15M) — eliminates per-poll Graph calls.
    • Pagination via @odata.nextLink in getBusyIntervals (do/while, merges per scheduleId).
    • Pre-flight conflict check removed from quickBook — relies on Graph 409 instead.
    • Sibling cache writes moved out of the outer cache->get() callback into a new setCacheValue() helper (avoids nested cache mutation).
    • New validateResourceAccess() deduplicates ~15 lines of config/permission/feed checks shared by quickBookOptions and quickBook.
  • Risk notes

    • 15-minute busy-intervals cache means availability shown in quickBookOptions can be up to 15 min stale; the actual booking call still 409s, so users get a correct error but an outdated picture.

@tuj tuj self-assigned this May 6, 2026
@tuj tuj added bug Something isn't working version 3 Issues related to the release of version 3 backlog Future fixes and improvements and removed version 3 Issues related to the release of version 3 labels May 6, 2026
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

API Specification - Non-breaking changes

API Changelog 1 vs. 1

API Changes

POST /v2/slides/{id}/action

  • added the new optional request property implementationClass (media type: text/html)
  • added the new optional request property implementationClass (media type: application/ld+json)
  • added the new optional request property implementationClass (media type: multipart/form-data)
  • the request property data became nullable (media type: application/ld+json)
  • the request property data became nullable (media type: multipart/form-data)
  • the request property data became nullable (media type: text/html)

@tuj tuj changed the title Cleans up issue with Instant Book caching Instant Book: Cleans up issue with caching May 6, 2026
@tuj
tuj requested a review from turegjorup May 7, 2026 10:15
@tuj tuj added version 3 Issues related to the release of version 3 and removed backlog Future fixes and improvements labels May 7, 2026
@tuj tuj changed the title Instant Book: Cleans up issue with caching fix: Instant Book: Cleans up issue with caching May 8, 2026

@turegjorup turegjorup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Bug fixes (interval boundary, timezone, spam-protect, duration validation, error mapping) and the DTO refactor look correct.

One non-blocking nit, pre-existing but in a file touched here: InteractiveServiceTest::testPerformAction lines 86–91 are unreachable — performAction() on line 84 throws and ends the test, so the second scenario never runs, and the expected message 'Action not allowed' doesn't match what InstantBook::performAction() throws ('Action not supported'). Worth splitting into two test methods or dropping the dead branch in a follow-up.

Comment thread src/InteractiveSlide/InstantBook.php Outdated
}

$url = $data['@odata.nextLink'] ?? null;
} while (null !== $url);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: Graph's @odata.nextLink is followed with GET, not a POST with the original body. Unlikely to fire on 1-hour windows, but if it ever does the second iteration will probably 4xx. Worth either a comment noting the assumption, or switching to GET (no body) when $url came from nextLink.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed after inspecting spec

@tuj
tuj merged commit 55482df into release/3.0.0 May 21, 2026
21 checks passed
@tuj
tuj deleted the feature/instantbook-cleanup branch May 21, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working version 3 Issues related to the release of version 3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants